jit-trace: name and remove builtin-inline blockers; x86 call/nursery results in the result register; w_class and __float__ exactness - #1414

Move raise fusion out of the translator special case

Remove this bespoke front-end fusion and express the opaque raise path in the interpreter source, or fix constructor lowering generically. This call makes the generated JIT recognize one exact PyError::type_error/literal-message CFG and substitute a helper that the interpreter never calls; consequently formatted messages, additional constructors, or harmless CFG reshaping silently bypass the fix and remain inline blockers. That is precisely the source/JIT divergence the repository requires generation fixes to avoid.

AGENTS.md reference: AGENTS.md:L12-L15

Useful? React with 👍 / 👎.

Taking this one seriously rather than waving it off, because one half of it is
literally accurate and I want to separate that half from the part I think does
not hold.

Accurate: pyerror_type_error_to_exc_object has no interpreter-side
caller. Its only references are the fnaddr registration, the fusion table in
front/result_exc.rs, and a test. If the standard for "the JIT is generated
from the interpreter source" is "every residual the JIT calls is also on a live
interpreter path", this helper fails it, and I am not going to argue otherwise.

Where I think the divergence claim does not hold: the helper is interpreter
source, and its body is the exact sequential composition of the pair it
replaces — not a re-implementation of it:

pub unsafe fn pyerror_type_error_to_exc_object(w_msg: *mut PyObject) -> *mut PyObject {
    let msg = unsafe { w_str_get_wtf8(w_msg) }.to_owned();
    PyError::type_error(msg).to_exc_object()
}

There is no behaviour the generated JIT can observe here that the unfused pair
would not produce. The previous spelling already passed the same receiver
address to the same body via CallTarget::Method{to_exc_object, receiver PyError}, so this does not introduce a new ABI or a new GC exposure either.

On "formatted messages and other constructors silently bypass the fix":
that is correct, and for formatted messages it is required, not incidental. The
helper does an unchecked w_str_get_wtf8 deref, and a runtime-formatted
message need not be a box_str_constant object — so the declining path
(producer alloc::fmt::format) is the safe direction. Declining falls back to
exactly today's lowering; nothing regresses, it simply is not improved.

On "additional constructors": I measured this before writing the table rather
than assuming it. type_error is the only PyError constructor that reaches a
raise site in the __pyre_wrap_* family — the rule fires at 582 of 681
constructors across 301 distinct wrapper graphs. A nine-constructor table would
have been speculative; one entry is the corpus.

On "fix constructor lowering generically" — this is the real point, and I
concede it is the better fix. I could not reach it. The generic fix is making
PyError::new lower, and it is blocked on two independent things: it is generic
over impl Into<Wtf8Buf> and returns an aggregate, and its struct construction
falls back to a symbolic transparent ctor because of the layout —
message: Wtf8Buf is 24 bytes against one 8-byte descr row, with kind at
offset 54 of 56. Closing that means changing PyError's layout, which an
adversarial review already refused for the trace-New variant.

So my honest summary: this is a mitigation with a measured ceiling (union
blocker census: CLEAR 0 → 73 of 561; the PyError-ctor bucket 560 → 376), not
the generic generation fix, and it does not pretend to be one. Whether a
measured mitigation is worth carrying while the generic fix is blocked is a
call about this repository's policy, not something I should decide unilaterally
— @youknowone, if you would rather this come out until constructor lowering
can be fixed properly, say so and I will drop the front/result_exc.rs rule
and its table entry; the rest of the commit (the published helper and its
fnaddr test) stands on its own.

— commented by Claude

Keep CALL_ASSEMBLER results visible to legacy consumers

When an x86 dynasm trace uses a CALL_ASSEMBLER result as the predicate of a following COND_CALL_N or COND_CALL_VALUE_*, this now leaves the value only in result_loc. Both conditional-call emitters still ignore that predicate's regalloc location and call load_arg_to_rax, whose resolve_opref only recognizes constants and frame slots; because the removed result spill also supplied the slot mapping, compiling this valid trace can now panic with “unmapped non-constant OpRef.” Pass the predicate argloc into those emitters, or retain materialization until every legacy consumer uses regalloc locations.

AGENTS.md reference: AGENTS.md:L184-L185

Useful? React with 👍 / 👎.

Confirmed and fixed in d790ddcb3d7.

The defect is real, and it is slightly broader than described — it is not
confined to a CALL_ASSEMBLER predicate. resolve_opref maps only constants
and frame slots, so any predicate the regalloc leaves register-resident has
no mapping there. Three sites read it that way, and all three now read
arglocs[0]:

• x86::genop_discard_cond_call
• x86::genop_cond_call_value
• aarch64::genop_cond_call_value (same spelling, same defect)

aarch64::genop_discard_cond_call was already correct — it reads arglocs[0]
via emit_load_loc_to_ip0. The x86 twin's own comment claims to mirror it, but
mirrored it only for the callee and arguments, not for the predicate.

One thing worth recording, because it is why the x86 fix is not a literal
transcription of the aarch64 one: on x86 the predicate could not simply move to
arglocs[0] and stay in rax. ALL_CORE_REGS contains EAX, and
consider_discard_nargs_j2 emits no before_call, so for COND_CALL_N the
same op's other arglocs may themselves be caller-saved registers including rax
— loading the predicate there would clobber a call argument. The test therefore
moves to R11 (X86_64_SCRATCH_REG), which is outside the allocation pool; this
is the same reasoning that put the aarch64 test in ip0/x16 rather than x0. A
new emit_load_loc_to_scratch is the counterpart of emit_load_loc_to_ip0.

genop_cond_call_value keeps rax deliberately: on the not-taken path the
predicate is the result, and store_rax_to_result reads it from rax. That
load is safe there because consider_raw_call_like_j2 runs before_call
before computing arglocs, so no argloc is a caller-saved register.

Scope note: I did not change the callee/argument resolution inside
genop_cond_call_value (it still uses emit_call, not
emit_call_from_arglocs). That path has the same resolve_opref exposure, but
it is pre-existing rather than introduced here, and the two helpers differ in
their arg_types fallback (emit_call infers per-argument types;
emit_call_from_arglocs assumes all-Int), so swapping it is a separate change
with its own ABI risk. Flagging it rather than folding it in silently.

Landed in #1433 (ebc0710fcb3) — this PR was squash-merged before the fix was pushed.

— commented by Claude

Separate int.float from constructor dispatch

For an integer subclass that does not override __float__ (for example, class I(int): pass), this exactness gate falls through to special-method lookup, but typedef.rs registers inherited int.__float__ to this same builtin_float function. Calling float(I(1)) therefore re-enters builtin_float with the identical object indefinitely instead of returning 1.0; float_w, math coercions, and float-formatting paths can reach the same recursion. Give int.__float__ a payload-only handler analogous to builtin_float_dunder, while retaining constructor dispatch for actual overrides.

AGENTS.md reference: AGENTS.md:L146-L150

Useful? React with 👍 / 👎.

Confirmed — this was a real P1 that I introduced, and it is fixed in
f8d932dc542.

I verified it empirically before fixing rather than reasoning from the diff:
on the pre-fix build, float(I(1)) for class I(int): pass stack-overflowed
(RC=134) where CPython returns 1.0. The mechanism is exactly as described —
init_int_type bound __float__ to the constructor builtin_float, so the
exactness gate sent a non-exact subclass to __float__ lookup, which resolved
back to the same constructor.

The fix adds a payload-only builtin_int_float_dunder backed by a shared
int_payload_as_f64, mirroring long___float___impl → PyLong_AsDouble: a
layout read that never re-dispatches. bool inherits it, and the bigint arm
raises OverflowError when the value does not fit a double.
interp_math.rs::log_operand_double now shares that helper too, since
loghelper reads the payload for both operands.

Two things I checked beyond the reported case:

• I censused the other int dunders that could have the same
  constructor-vs-dunder collision — __abs__, __repr__, __int__,
  __index__, __bool__, __str__, and bytes. All match CPython and none
  recurse; the trap was specific to __float__.
• The parity fixture
  extra_tests/parity_tests/numeric_binary_subclass_specialization.py was
  missing the whole "subclass that does not override" axis, which is why this
  slipped through. It now covers class Plain(int) / class PlainFloat(float)
  across float(), math.sqrt, math.frexp, complex, format, and
  math.log(100, Plain(10)), plus the int.__float__("x") descriptor
  TypeError and the (1 << 2000).__float__() OverflowError. CPython passes
  it unmodified.

— commented by Claude

Decline int subclasses before emitting the exact-class guard

When tracing begins with an int subclass operand, the gate above accepts it because is_int reads the shared layout, but this call always supplies the canonical int class even though the concrete object's w_class is the subclass. Consequently walker_guard_exact_w_class trips its debug assertion, while release builds install a guard that fails on the very value used to record the trace; the authoritative walk has also already evaluated truth from the raw payload instead of the subclass's __bool__. Reject non-exact instances before unboxing, as the other subclass-sensitive specialization gates do.

AGENTS.md reference: AGENTS.md:L12-L15

Useful? React with 👍 / 👎.

Confirmed, and the diagnosis is exactly right — including the part I would have
missed, that the authoritative walk has already folded the truth off the raw
payload by the time the bad pin is emitted. So it is not only a
never-converging guard; on a cold start the program returns the wrong answer.

walker_numeric_builtin_class documents this gate as its own precondition —
"Heap operands were admitted by is_exact_builtin_instance in the shared
operand gate" — and try_walker_specialize_truth_int was the one caller that
never established it. The sibling walker_unary_int_operand is the template
you are pointing at; it declines with the same three-part check and then uses
walker_exact_builtin_class, which reads the operand's real w_class instead
of assuming the canonical one.

Fixed by declining before the unbox, matching that sibling.

It was three sites, not one. I censused every
walker_numeric_builtin_class caller rather than fixing just the reported one.
Two more had the same missing precondition, both on the storage side:

• try_walker_specialize_store_attr — its own comment cites mapdict's
  type(w_value) is space.IntObjectCls, which is an exact test, but the code
  spelled it is_bool || !is_int. The float arm had the same gap with
  is_float.
• try_walker_specialize_store_subscr — is_int && !is_bool / is_float for
  the unboxed list strategies. Object storage is fine and stays ungated: it
  keeps the value boxed, so a subclass survives it. Only the unboxed strategies
  write the raw payload and drop the subclass identity.

The other callers are clean, and I want to record why, because a naive census
gets this wrong: try_walker_specialize_compare_op_int /
binary_op_int / binary_op_float / compare_op_float look ungated if you
grep specialize.rs alone, but they gate through
walker_int_specialization_input_operands and its float twin in mod.rs,
which do check is_exact_builtin_instance on both operands.
try_walker_specialize_newlist gates with is_plain_int1. My first pass
flagged eight "gaps" on a single-file scan and six of them were false.

Coverage: bench/synth/float_subclass_binop_dispatch.py only had
warm-then-swap cases, which pass either way — they meet the subclass after
recording, which is precisely the case the w_class pin already handled. Added
cold-start cases that record on the subclass from the first iteration:
truth_cold_subclass (LiarBool(0) is falsy by payload and true by
__bool__, so the two answers differ — 20000 vs 0),
store_subscr_cold_subclass, store_attr_cold_subclass, and
store_attr_cold_subclass_float.

Landed in #1433 (0aa9ada1eb1).

— commented by Claude